Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes .proto formatting across the repo by adopting buf format, updates CI/local checks to enforce the formatting, and applies the formatter output to existing proto sources (including regenerated Go outputs where applicable).
Changes:
- Add a pinned
buf-based formatter script plusmake proto-fmt/make proto-fmt-checktargets. - Update check/codegen scripts to incorporate formatting checks and handle
buf’s multi-line option formatting for C++ generation. - Reformat many
.protofiles and update generated Go descriptor blobs accordingly.
Reviewed changes
Copilot reviewed 56 out of 61 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/proto_format.sh | New pinned buf download + buf format runner for --check/--write. |
| scripts/check.sh | Runs proto formatting check as part of make check; adds proto option checks (currently problematic). |
| scripts/common.sh | Reworks clean_gogo_proto to strip gogoproto options robustly under buf formatting. |
| scripts/generate_cpp.sh | Uses clean_gogo_proto per file after copying protos for C++ generation. |
| Makefile | Adds proto-fmt and proto-fmt-check targets wired to the formatter script. |
| README.md | Documents the new formatting workflow and how to fix formatting failures. |
| .editorconfig | Enforces 2-space indentation for *.proto files in editors. |
| proto/tsopb.proto | Reformatted via buf format. |
| proto/tracepb.proto | Reformatted via buf format. |
| proto/schedulingpb.proto | Reformatted via buf format. |
| proto/resource_usage_agent.proto | Reformatted via buf format. |
| proto/resource_manager.proto | Reformatted via buf format. |
| proto/replication_modepb.proto | Reformatted via buf format. |
| proto/recoverdatapb.proto | Reformatted via buf format. |
| proto/raft_serverpb.proto | Reformatted via buf format. |
| proto/raft_cmdpb.proto | Reformatted via buf format. |
| proto/mpp.proto | Reformatted via buf format. |
| proto/metapb.proto | Reformatted via buf format. |
| proto/meta_storagepb.proto | Reformatted via buf format. |
| proto/logbackuppb.proto | Reformatted via buf format. |
| proto/keyspacepb.proto | Reformatted via buf format. |
| proto/import_kvpb.proto | Reformatted via buf format. |
| proto/gcpb.proto | Reformatted via buf format. |
| proto/errorpb.proto | Reformatted via buf format. |
| proto/enginepb.proto | Reformatted via buf format. |
| proto/encryptionpb.proto | Reformatted via buf format. |
| proto/disk_usage.proto | Reformatted via buf format. |
| proto/disaggregated.proto | Reformatted via buf format. |
| proto/diagnosticspb.proto | Reformatted via buf format. |
| proto/debugpb.proto | Reformatted via buf format. |
| proto/deadlock.proto | Reformatted via buf format. |
| proto/coprocessor.proto | Reformatted via buf format (notably multi-line field options formatting). |
| proto/configpb.proto | Reformatted via buf format. |
| proto/cdcpb.proto | Reformatted via buf format. |
| proto/autoid.proto | Reformatted via buf format. |
| include/rustproto.proto | Reformatted via buf format. |
| include/eraftpb.proto | Reformatted via buf format. |
| pkg/logbackuppb/logbackuppb.pb.go | Updated generated descriptor blob due to proto formatting/re-ordering effects. |
| pkg/keyspacepb/keyspacepb.pb.go | Updated generated descriptor blob due to proto formatting/re-ordering effects. |
| pkg/configpb/configpb.pb.go | Updated generated descriptor blob due to proto formatting/re-ordering effects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Just to avoid the formatting causing some problems, I want to wait some weeks after master branch's merging before continue release branch formatting. |
dc4a121 to
bdcab1d
Compare
Signed-off-by: lance6716 <lance6716@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
After about a month, TiDB has upgraded kvproto to the version after #1408, and performance regression test does not show any problems. So I think we have confidence that the formatter is not harmful.
To reduce conflicts whtn cherry-pick PR from master branch to release-8.5, I propose that we also add the formatter to release-8.5
this PR can be reproduced by
bufas formatter #1408git show e75b531a1914ff517d3945ac61ca93072db242c4 -- Makefile scripts/proto_format.sh scripts/common.sh scripts/generate_cpp.sh .editorconfig README.md | git apply --3wayfor
scripts/check.shfile, there's a conflict. So manually edit like